Skip to content

Use abd in dbuf and arc_buf to allow buffer sharing - #18854

Open
tuxoko wants to merge 2 commits into
openzfs:masterfrom
tuxoko:abd_share
Open

Use abd in dbuf and arc_buf to allow buffer sharing#18854
tuxoko wants to merge 2 commits into
openzfs:masterfrom
tuxoko:abd_share

Conversation

@tuxoko

@tuxoko tuxoko commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

For uncompressed block, we were supposed to allow buffer sharing between hdr->b_pabd and arc_buf->b_data. However, we can only do so for linear buffer as arc_buf and dbuf doesn't work with abd. And in practice, b_pabd will almost always be scatter type abd except for small buffer, so buffer sharing almost never happens.

In this change we make arc_buf and dbuf use abd instead of raw buf, and make them use scatter abd for user data. This allows us to share the abd between b_pabd, arc_buf and dbuf for data. This reduces memory copy needed for both read and write, which can be a bottleneck for high performance setup.

For metadata, we only use linear abd, so the user of metadata dbuf will need to use abd_to_buf to access the data, but otherwise it's pretty much the same as before.

Motivation and Context

Description

How Has This Been Tested?

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist

@tuxoko

tuxoko commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Note, I hit an VERIFY failure in this test rsend/send_realloc_encrypted_files.ksh

VERIFY3U(soff + size, <=, sabd->abd_size) failed (2048 <= 1536)
PANIC at abd.c:989:abd_iterate_func2()
Showing stack for process 3047098
CPU: 5 UID: 0 PID: 3047098 Comm: receive_writer Kdump: loaded Tainted: P           OE       7.0.0-14-generic #14~24.04.3-Ubuntu PREEMPT(lazy)                                      Tainted: [P]=PROPRIETARY_MODULE, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
Hardware name: Nutanix AHV, BIOS nutanix-ahv-4.20230302.1.2662.el8 04/01/2014
Call Trace:
 <TASK>
 dump_stack_lvl+0x76/0xa0
 dump_stack+0x10/0x20
 spl_dumpstack+0x28/0x40 [spl]
 spl_panic+0xfb/0x120 [spl]
 ? abd_update_linear_stats+0x40/0xd0 [zfs]
 ? abd_alloc_linear_impl+0x4b/0xb0 [zfs]
 abd_iterate_func2+0x48c/0x520 [zfs]
 ? arc_get_data_abd+0x117/0x300 [zfs]
 ? __pfx_abd_copy_off_cb+0x10/0x10 [zfs]
 abd_copy_off+0x17/0x30 [zfs]
 dbuf_new_size+0xc3/0x3a0 [zfs]
 dbuf_spill_set_blksz+0x5a/0xf0 [zfs]
 receive_writer_thread+0xc41/0xf00 [zfs]
...

However, we didn't hit this in our 2.3 branch. (actually we have some unrelated change masked this issue)
And this seems to be an existing issue.
I add some debug code on current master and can see similar size discrepancy
ddd.patch

[  965.917568] ZTS run /usr/local/share/zfs/zfs-tests/tests/functional/rsend/send_realloc_encrypted_files.ksh
[  985.320951] WARNING: abd_size=1536 osize=2048 size=2560 shared=0

@amotin

amotin commented Jul 28, 2026

Copy link
Copy Markdown
Member

As you have said, ARC is typically built from scatter, not mapped into KVA buffers, that is good for memory fragmentation, but hard to access. Dbufs same time are linear and KVA-mapped for easy access, but we can't use them for everything, since we quickly get out of KVA due to fragmentation. What you haven't said is where do you expect them to meet.

@tuxoko

tuxoko commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

@amotin
Sorry I wasn't clear enough. In this change we use abd for all dbuf, but we only use scatter abd for user data. Metadata dbufs will use linear abd, and the users of metadata dbufs will now access it with abd_to_buf, but otherwise will behave practically the same. Metadata is always compressed so there's no buffer sharing anyway.

It's possible to use scatter abd for some types of dbuf like bp and dnode blocks where the structure is well aligned and won't cross any page boundary. It will be a trade off of memory fragmentation vs extra memcpy for compression and extra code complexity.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jul 28, 2026
@tuxoko

tuxoko commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

The decrypt_fault failure is not caused by this change specifically, but seems to happen when buffer sharing is enabled.
In #17233 , I tried disabling scatter abd to force buffer sharing and it also failed.
I don't really know what's the issue though.

@tuxoko
tuxoko force-pushed the abd_share branch 2 times, most recently from 38b49df to 05f1eaa Compare July 30, 2026 00:31
receive_spill will call dbuf_spill_set_blksz, which in turn calls
dbuf_new_size when resizing spill block. However, receive_spill holds
a raw buf, which can be compressed and less than db_size. This causes
an out-of-bound access when dbuf_new_size tries to copy the data using
db_size as length.

To fix this, we just disable the copy when coming from receive_spill,
since we will assign a new arc_buf later anyway.

Signed-off-by: Chunwei Chen <david.chen@nutanix.com>
For uncompressed block, we were supposed to allow buffer sharing between
hdr->b_pabd and arc_buf->b_data. However, we can only do so for linear
buffer as arc_buf and dbuf doesn't work with abd. And in practice,
b_pabd will almost always be scatter type abd except for small buffer,
so buffer sharing almost never happens.

In this change we make arc_buf and dbuf use abd instead of raw buf, and
make them use scatter abd for user data. This allows us to share the abd
between b_pabd, arc_buf and dbuf for data. This reduces memory copy
needed for both read and write, which can be a bottleneck for high
performance setup.

For metadata, we only use linear abd, so the user of metadata dbuf will
need to use abd_to_buf to access the data, but otherwise it's pretty
much the same as before.

Signed-off-by: Chunwei Chen <david.chen@nutanix.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Code Review Needed Ready for review and testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants